Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Outreachy] Removed ipv6 fallback #1900

Merged
merged 1 commit into from
Nov 15, 2018
Merged

[Outreachy] Removed ipv6 fallback #1900

merged 1 commit into from
Nov 15, 2018

Conversation

tanushree27
Copy link

  • Replaced ipv6_* callers by non-prefixed versions.
  • Removed ipv6_* pointer functions and stub functions.
  • Reverted ensure_socket_initialization.

@tanushree27 tanushree27 reopened this Oct 27, 2018
@tanushree27 tanushree27 changed the title Remove ipv6 fallback [Outreachy] Removed ipv6 fallback Oct 27, 2018
compat/mingw.c Show resolved Hide resolved
compat/mingw.c Show resolved Hide resolved
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that the code works as expected, and I suggested a couple of changes, so we're close.

compat/mingw.c Show resolved Hide resolved
@tanushree27
Copy link
Author

@dscho squashed commits and revised the commit message.

@dscho
Copy link
Member

dscho commented Nov 5, 2018

I just started a rebuild, as I am worried that the loooong run time of the test suite might be somehow caused by this patch.

Admittedly, I was unable to spot anything wrong, from my perspective this PR should be ready (and pass the test suite in the regular time).

@dscho
Copy link
Member

dscho commented Nov 6, 2018

Failing after 240m — Build #23457 has failed

That's really bad. And it is consistently failing with this PR, for the same reason: it times out after 4h (!!!).

Quite honestly, I have no idea why/where the problem lies, but since it happens with this PR only, it must be caused by the changes introduced by this PR... Very strange.

To support IPv6, Git provided fall back functions for Windows versions that
did not support IPv6. However, as Git dropped support for Windows XP and
prior, those functions are not needed anymore.

Removed those fallbacks by reverting commit[1] and using the functions
directly (without 'ipv6_' prefix).

[1] fe3b2b7.

Signed-off-by: tanushree27 <[email protected]>
@dscho
Copy link
Member

dscho commented Nov 14, 2018

Ah! In 28f1d8e you removed the shims altogether, and with them, the socket initialization. I would wager a guess that that makes the difference, and that we do not even need to initialize the socket system to look up IPv6 addresses? But why, then, do we look up IPv6 addresses so often that it slows down the test suite noticeably?

@tanushree27
Copy link
Author

In 8cf11a9 I commented out shims, and this worked like a charm build #20181113.7. However, when I removed the shims in 28f1d8e the tests suite failed.
Later I realised that in C // is not used for comments, so what exactly happened in 8cf11a9?

@dscho
Copy link
Member

dscho commented Nov 14, 2018

when I removed the shims in 28f1d8e the tests suite failed.

You refer to https://git-for-windows.visualstudio.com/git/_build/results?buildId=24879&view=logs, correct?

If so, the linux-gcc phase's failure strikes me as a flakey test (we have some of those, sadly), and the Windows phase fails in a test that is unrelated to IPv6, so I would assume that that is also a flakey test...

Could you re-push 28f1d8e?

@tanushree27
Copy link
Author

You refer to https://git-for-windows.visualstudio.com/git/_build/results?buildId=24879&view=logs, correct?

Yes, I was referring to this.

Could you re-push 28f1d8e?

Done.

@dscho
Copy link
Member

dscho commented Nov 15, 2018

Could you re-push 28f1d8e?

Done.

And now the tests pass ;-)

@dscho dscho merged commit b517370 into git-for-windows:master Nov 15, 2018
@dscho dscho added this to the v2.19.1(2) milestone Nov 15, 2018
dscho added a commit to dscho/git that referenced this pull request Nov 19, 2018
…fallback

[Outreachy] Removed ipv6 fallback
dscho added a commit to dscho/git that referenced this pull request Nov 20, 2018
…fallback

[Outreachy] Removed ipv6 fallback
dscho added a commit that referenced this pull request Nov 21, 2018
dscho added a commit that referenced this pull request May 7, 2019
dscho added a commit that referenced this pull request May 7, 2019
git-for-windows-ci pushed a commit that referenced this pull request May 8, 2019
dscho added a commit that referenced this pull request May 9, 2019
dscho added a commit that referenced this pull request May 9, 2019
git-for-windows-ci pushed a commit that referenced this pull request May 9, 2019
dscho added a commit that referenced this pull request May 10, 2019
dscho added a commit that referenced this pull request May 10, 2019
git-for-windows-ci pushed a commit that referenced this pull request May 10, 2019
git-for-windows-ci pushed a commit that referenced this pull request May 10, 2019
dscho added a commit that referenced this pull request May 13, 2019
dscho added a commit to dscho/git that referenced this pull request May 13, 2019
…fallback

[Outreachy] Removed ipv6 fallback
dscho added a commit to dscho/git that referenced this pull request May 13, 2019
…fallback

[Outreachy] Removed ipv6 fallback
git-for-windows-ci pushed a commit that referenced this pull request May 14, 2019
dscho added a commit that referenced this pull request May 14, 2019
dscho added a commit that referenced this pull request May 14, 2019
dscho added a commit that referenced this pull request May 21, 2019
dscho added a commit that referenced this pull request May 22, 2019
dscho added a commit that referenced this pull request May 25, 2019
dscho added a commit that referenced this pull request Jun 4, 2019
dscho added a commit that referenced this pull request Jun 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants